Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update for Chart.JS 3.x Support #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lincolnthree
Copy link

@lincolnthree lincolnthree commented May 8, 2020

First, great plugin, thank you!

This pull request updates this library for support of the upcoming Chart.JS 3.x release (currently in alpha.)

See documentation here:
https://www.chartjs.org/docs/next/getting-started/v3-migration

@stockiNail
Copy link

@lincolnthree I think this PR doesn't work with Chart.js 3.0.0 beta.

To register the plugin, the call is changed:

Chart.register(plugin);

instead of

Chart.plugins.register(plugin);

Furthermode the multilabels does not work because they are configured as an array. See issue #7754 in Chart,js.

With the current code, the optionParm argument of beforeUpdate method of plugin is an empty object, if you configured your plugin to manage multiple labels.

In my plugin code, I have changed, taking the configuration from chart instance instead of using the option argument, like that:

beforeDatasetsUpdate: function (chart) {
   if (!SUPPORTED_TYPES[chart.config.type]) {
      return;
   }
   const optionParams = chart.options.plugins.labels;
   var options = optionParams;
   if (!Array.isArray(optionParams)) {
      options = [optionParams];
   }
   ....
   ....

@lincolnthree
Copy link
Author

lincolnthree commented Sep 7, 2020

That's probably right, since this PR was created before the .beta was released :) And since it doesn't seem like it's going to be merged, care to send a PR to my branch? I can pull it in and update it, which will also update this PR.

@jipiboily
Copy link

Hey folks! Would love to see v3 support being merged and released.

What's the current blocker and how can we help it?

@jipiboily
Copy link

@emn178 I don't want to be rude; I'm just curious: are you still maintaining the product available to review this PR maybe so that we can know what to collectively work on to get it merged and released with ChartJS v3 support? Would be awesome! :)

Thanks!

@jipiboily
Copy link

@lincolnthree is this branch working for you in production right now?

It doesn't seem to work for me, but I have a weird setup with ChartjsNodeCanvas so it might totally be due to something in my setup.

@lincolnthree
Copy link
Author

@jipiboily No. The v3 pre-release branch in my PR is no longer compatible and requires more changes to become current with Chart.is 3.x release.

@jipiboily
Copy link

jipiboily commented May 19, 2021

Thanks for the reply @lincolnthree. I'll see if / when I can get to figure this out. I'll update here if / when that happens.

Or did you find another plugin that works with v3 and does something similar?

@DavideViolante
Copy link

DavideViolante commented May 19, 2021

It doesn't seem to work, even using this PR and the changes proposed by @stockiNail in his first comment.
What else should be changed to make it work with v3? Can we figure it out all together?
Ref: https://www.chartjs.org/docs/next/getting-started/v3-migration.html

edit: made it work #140

@dana-mb
Copy link

dana-mb commented Jun 4, 2021

@DavideViolante, thank you for updating the plugin!
I've encountered a problem.. how can I use two labels?

This:
labels: [
{
render: 'label',
position: 'outside'
},
{
render: 'value'
}
]
Isn't working with the new chart.js.

@DavideViolante
Copy link

Thanks for pointing this out @dana-mb
It seems like it doesn't work when mixing ouside and inside options. Outside or inside alone works.
I don't know at the moment how to fix it, PRs are welcome on my fork...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants